fix: Fix Auto Approve UI Closing #2579 #4395
Conversation
This PR addresses issue RooCodeInc#2579, where the auto-approve toast notification would reappear even after the user selected "Don't show again." The root cause was that the user's preference was not being persisted across sessions.
dosubot
bot
added
size:M
Mnehmos
changed the title
daniel-lxs
left a comment
daniel-lxs
left a comment
There was a problem hiding this comment.
Hey @Mnehmos, Thank you for working on this fix!
I took a look at your PR and left a couple of suggestions that are worth taking a look at.
Let me know if you have any questions!
| // If the user unchecks the last enabled option, autoApprovalEnabled should become false. | ||
| // This logic is already handled by the main checkbox's disabled state in the collapsed view. | ||
| // So, we only need to ensure it turns ON when an individual is turned ON. | ||
| } |
There was a problem hiding this comment.
The state synchronization logic here seems complex and the extensive comments suggest uncertainty about the approach. Could we simplify this by creating a more robust state update mechanism?
Specifically, the else branch (lines 84-95) acknowledges that it doesn't handle the case where disabling an action should potentially disable the main checkbox. This could lead to inconsistent UI states.
Consider extracting this logic into a separate function that can properly calculate the new state based on all current toggle values, rather than relying on "future render cycles".
| if (value === true) { | ||
| setAutoApprovalEnabled(true) | ||
| vscode.postMessage({ type: "autoApprovalEnabled", bool: true }) | ||
| } else { |
There was a problem hiding this comment.
The logic here only handles enabling the main checkbox when an individual action is enabled, but doesn't handle the reverse case. When the last enabled action is disabled, the main checkbox should also be disabled automatically.
Consider adding logic to check if all toggles are now false and update autoApprovalEnabled accordingly:
if (value === true) {
setAutoApprovalEnabled(true)
vscode.postMessage({ type: "autoApprovalEnabled", bool: true })
} else {
// Check if this was the last enabled toggle
const updatedToggles = { ...toggles, [key]: false }
const hasAnyEnabled = Object.values(updatedToggles).some(v => !!v)
if (!hasAnyEnabled) {
setAutoApprovalEnabled(false)
vscode.postMessage({ type: "autoApprovalEnabled", bool: false })
}
}There was a problem hiding this comment.
Were these changes accidental?
There was a problem hiding this comment.
Yes yes and yes. This was my first PR attempt, I'll review all of these and come with a much more solid approach. Thank you for reviewing these changes.
daniel-lxs
moved this from PR [Needs Prelim Review]
to PR [Changes Requested]
in Roo Code Roadmap
dosubot
bot
added
size:XL
…ssues - Remove master toggle UI from AutoApproveSettings component - Remove master toggle translation keys from English locale - Update tests to remove master toggle functionality - Fix useDeepCompareEffect warning in ChatView.tsx by using useEffect - Optimize useAutoApproveState hook for better performance: - Improve memoization dependencies - Batch state updates in handleMasterToggle - Optimize callback dependencies - Optimize AutoApproveMenu with memoized setters object - Resolves language check failure in PR RooCodeInc#4395
daniel-lxs
moved this from PR [Changes Requested]
to PR [Needs Prelim Review]
in Roo Code Roadmap
|
Hey @Mnehmos, Thank you for taking a look at the suggestions! I was testing the auto-approve buttons and noticed a weird behavior: 2025-06-11_08-01-55.mp4It seems like the buttons are enabled or disabled randomly, can you take a look? |
daniel-lxs
moved this from PR [Needs Prelim Review]
to PR [Draft / In Progress]
in Roo Code Roadmap
github-project-automation
bot
moved this from PR [Draft / In Progress]
to Done
in Roo Code Roadmap
3 participants
Related GitHub Issue
Closes: #2579
Description
This pull request re-implements the changes from the original PR #4371.
This PR addresses issue #2579, which pointed out confusing UI behavior in the auto-approve feature. The changes ensure that the UI state is always clear and unambiguous to the user.
The primary changes are:
webview-ui/src/components/chat/AutoApproveMenu.tsx:webview-ui/src/components/settings/AutoApproveToggle.tsx:.gitignore:webview-ui/node_modulesandwebview-ui/distto the ignore list.These changes ensure that the user's preference is respected, improving the overall user experience.
Test Procedure
Type of Change
srcor test files.Pre-Submission Checklist
npm run lint).console.log) has been removed.npm test).mainbranch.npm run changesetif this PR includes user-facing changes or dependency updates.Screenshots / Videos
Documentation Updates
Additional Notes
This PR is a recreation of #4371 after the original branch was accidentally deleted.
Important
Fixes UI behavior in
AutoApproveMenu.tsxandAutoApproveToggle.tsxto ensure clear auto-approve state indication and updates.gitignore.AutoApproveMenu.tsx, the main "Auto-Approve" checkbox is disabled when no actions are approved and is automatically enabled when any action is selected.AutoApproveToggle.tsx, toggle buttons reflect their individual state and indicate if the overall auto-approve setting is enabled.webview-ui/node_modulesandwebview-ui/distto.gitignore.This description was created by
for 629aee0. You can customize this summary. It will automatically update as commits are pushed.